could we also run it with neutrino? as the implementation would suggest it works there

good point — it does work on neutrino, but only against a package-aware miner. neutrino has no mempool, so SubmitPackage just broadcasts parent+child and leans on the peer's 1p1c relay — that needs the bitcoind miner (minerbackend=bitcoind, v28+); against the default btcd miner the zero-fee parent is simply rejected and never assembles. so a neutrino row would have to be gated on minerbackend=bitcoind and assert confirmation (the neutrino result itself is a synthetic best-effort one). afaict there's no backend=neutrino+minerbackend=bitcoind combo in CI today, so it'd be skipped everywhere unless we add that job. happy to add the gated test (+ the CI combo) if you think it's worth it — wdyt?

🟠 Major F2: Neutrino SubmitPackage is not atomic and can report unverifiable success

The neutrino branch loops PublishTransaction(tx, "") over txns individually, ignores maxFeeRate entirely (no fee ceiling is ever applied on neutrino), and on success synthesizes PackageMsg: "success". This breaks the package-atomicity contract that the whole feature depends on: a zero-fee v3 parent broadcast standalone is invalid, so either the call errors at the parent (the feature does not work on neutrino) or — if an earlier tx publishes and a later one fails — the mempool is left with a partial, non-atomic subset with no rollback. When every tx happens to publish, the synthesized "success" asserts a package-accept verdict a light client cannot substantiate, so downstream logic (sweeps, force-close bumps) may proceed on a false premise. On failure the loop returns a bare err with no index/txid, so the partial-broadcast state is undiagnosable. The neutrino path should either return an explicit "unsupported/unverified" result or, at minimum, not claim "success" and should wrap publish errors with the offending tx.

(F2) Right — neutrino has no mempool, so it can't substantiate a package-accept verdict. It does a best-effort individual P2P broadcast (relying on the peer's 1p1c relay) and returns a synthetic result; only bitcoind performs real submitpackage. The proto + method docs now call this out explicitly as best-effort/non-atomic. Happy to return a distinct non-"success" package_msg for the neutrino path if you'd prefer a clearer signal.

Addressed: the neutrino branch is removed — BtcWallet.SubmitPackage now returns ErrUnimplemented for both btcd and neutrino (only bitcoind does real submitpackage). It no longer fabricates a "success" verdict a light client can't substantiate, and there's no partial-broadcast path (the old loop would have failed at the zero-fee parent anyway, since it's below min-relay-fee on its own). Callers that want neutrino relay broadcast the txns individually and lean on the peer's 1p1c relay. Proto + method docs updated to match.

Update (after discussion with @yyforyongyu, went with the best-effort option rather than erroring): the neutrino path still broadcasts each tx individually and relies on the peer's 1p1c relay, but it no longer claims "success" — it returns a deliberately non-success PackageMsg ("neutrino best-effort broadcast; acceptance unverified") and wraps publish errors with the offending tx index + txid, so callers treat it as an unverified broadcast, not a package-accept verdict.

Shortened the neutrino PackageMsg to "broadcast-unverified" (the rationale is in the const's doc comment) — less verbose than the first pass.

🟠 Major F3: No bound on package size; unbounded raw_txs decode

The handler accepts req.RawTxs with no cap on count or per-tx size, pre-allocating make([]*wire.MsgTx, 0, len(req.RawTxs)) and calling tx.Deserialize on every element before any validity check. wire.MsgTx.Deserialize honors length prefixes in the byte stream and allocates per-input/output slices accordingly, so a small request body can drive large allocations (bandwidth amplification). While onchain:write gates this, that macaroon can be delegated/RBAC-scoped, so an authenticated-but-not-operator caller can force outsized work. A real package is at most a parent+child (bitcoind caps packages at 25); reject when len(req.RawTxs) exceeds a small bound and cap individual raw-tx size before deserializing. Documenting the cap in the proto/lncli would also make the limit part of the published contract.

Done (F3) — the handler now rejects packages larger than maxPackageTxns = 25 (mirroring bitcoind's MAX_PACKAGE_COUNT) before deserializing, bounding the work an authenticated caller can force.

🟠 Major F4: Package rejection isn't surfaced as an error; success is a free-form string

Whole-package acceptance is conveyed only through PackageMsg, a free-form string the caller must compare against the literal "success" (the itest does require.Equal(ht, "success", ...)). The handler returns a nil gRPC error whenever the backend call returns no Go error, even if bitcoind rejected the package (PackageMsg != "success") or individual txns failed (tx_results[*].error populated) — so a caller that checks only the gRPC status believes an on-chain funds operation succeeded when it did not. Per-tx errors are likewise opaque strings, so a caller cannot machine-distinguish "fee too low" from "missing inputs" from "already in mempool" to decide its next action. Add a typed status (e.g. an enum: fully-accepted / partially-accepted / rejected) and reserve package_msg for human-readable detail; a brittle string contract on a public RPC is hard to change later.

(F4) Per-tx failures are surfaced via tx_results[*].error and package_msg carries bitcoind's verdict, mirroring the submitpackage RPC's own result shape, so a fully-rejected package returns the detail in the response rather than a gRPC error. If we want callers to machine-distinguish accepted/partial/rejected without parsing package_msg, I can add a typed status enum (or return a gRPC error on non-acceptance) — lmk which you'd prefer.

🟡 Minor F9: Fee-rate conversion is correct but untested, with magic constant and integer-only granularity

btcPerKvB := float64(*maxFeeRate) * 1000 / btcutil.SatoshiPerBitcoin is dimensionally and numerically correct (1 sat/vByte → 0.00001 BTC/kvB; bitcoind's 0.10 BTC/kvB default = 10 sat/vByte), but: there is no unit test pinning a known input, so an off-by-1000 regression here would silently relax or tighten the user's fee ceiling; the 1000 (vByte→kvB) is an inline magic number; and because chainfee.SatPerVByte is integer, only whole sat/vByte ceilings are expressible and very large values lose integer precision once past 2^53 in the float64 product. Add a conversion unit test, name the constant, and document the integer-granularity limitation on the parameter.

Done (F9) — extracted satPerVByteToBTCPerKvB with a named vBytesPerKvB constant (documenting the integer-granularity limitation) and added a unit test pinning the known reference points.

This comment is inaccurate for two backends. The btcd backend in btcwallet v0.18.0 returns ErrUnimplemented for SubmitPackage, so it does not use submitpackage. Neutrino also does not provide atomic validation/acceptance; it can only attempt non-atomic individual P2P broadcasts. The RPC docs should probably say bitcoind-only for real package submission, and clearly mark neutrino as best-effort/non-atomic if it remains supported.

Done — reworded so it's clear only the bitcoind backend performs real package submission (via submitpackage); the btcd backend returns an error, and neutrino is best-effort/non-atomic (individual P2P broadcast relying on 1p1c relay).

Same backend-support issue here: this says bitcoind/btcd, but the btcwallet v0.18.0 btcd backend returns ErrUnimplemented for SubmitPackage. This should be narrowed to bitcoind unless btcd support lands.

Done — narrowed the doc: only bitcoind does real submission; the btcd backend has no submitpackage handler and returns ErrUnimplemented.

This type assertion is always true now that SubmitPackage was added to the lnwallet.WalletController interface and w.cfg.Wallet is statically a WalletController. Either keep SubmitPackage off the base interface and make this an optional capability, or remove the local interface/assertion and call w.cfg.Wallet.SubmitPackage directly.

Done — SubmitPackage is on WalletController now, so the assertion was always true. Removed the packageSubmitter interface + assertion and call w.cfg.Wallet.SubmitPackage directly (also dropped the now-unused btcjson import).

These replace-allow-list entries look stale now that go.mod no longer has a replace for btcwallet/btcd. Since the preceding comment points readers to go.mod for the reason, please remove these draft-era allow-list entries unless a replace is still expected before merge.

Done — removed the stale btcwallet/btcd replace-allow-list entries; go.mod no longer replaces them.

think we should address https://github.com/lightningnetwork/lnd/pull/10900/changes#r3493050462 here - maybe name it published or broadcast-unverified? just to make it clear at the callsite so it can check